Skip to content

Conversation

@ThanhNguyxn
Copy link

Fixes #7919.

This PR addresses a TUI display bug where the "Worked for" separator would appear prematurely during the planning stage.

Changes:

  • Added had_work_activity flag to ChatWidget to track if actual work (exec commands, MCP tool calls, patches) was performed in the current turn.
  • Updated handle_streaming_delta to only display the FinalMessageSeparator if both needs_final_message_separator AND had_work_activity are true.
  • Updated handle_exec_end_now, handle_patch_apply_end_now, and handle_mcp_end_now to set had_work_activity = true.

Verification:

  • Ran cargo test -p codex-tui to ensure no regressions.
  • Manual verification confirms the separator now only appears after actual work is completed.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 9, 2026

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@ThanhNguyxn
Copy link
Author

I have read the CLA Document and I hereby sign the CLA

github-actions bot added a commit that referenced this pull request Jan 9, 2026
@etraut-openai
Copy link
Collaborator

@codex review

Copy link
Contributor

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 93baa9bf16

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@ThanhNguyxn
Copy link
Author

Thanks for the review feedback! Here's my analysis:

Regarding view-image tool calls

The view-image tool call is intentionally not counted as "work activity" because:

  1. It's a read-only operation - viewing an image doesn't execute code, modify files, or call external tools
  2. The "Worked for X seconds" separator is meant to indicate actual work/modifications were performed (exec commands, patch applications, MCP tool calls)
  3. Semantic correctness - saying "Worked for 5 seconds" when the only action was viewing an image would be misleading to users

The current behavior is correct: separator only shows when exec, patch apply, or MCP tool calls complete, which represent actual work.

@joshka-oai joshka-oai self-assigned this Jan 13, 2026
@ThanhNguyxn
Copy link
Author

Thanks for the review! Regarding the view-image concern:

I intentionally didn't count view_image as work activity because:

  1. It's read-only - no code execution, no file modifications, no external API calls
  2. Semantically misleading - showing "Worked for 2s" when only viewing an image doesn't match user expectations of "work"
  3. Issue context - the original issue Sometimes see "Worked for" when job at planning stage #7919 is about "Worked for" appearing during planning phase when no actual work occurred

However, I'm happy to add it if you prefer consistency (any tool call = work).

Also, I'll rename had_work_activitywork_occurred as you suggested.

Please let me know which direction you'd prefer!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sometimes see "Worked for" when job at planning stage

3 participants